Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch status of all scenarios in DM in a single operation instead of separately for each scenario. #7295

Merged
merged 22 commits into from
Dec 19, 2024

Conversation

mgoworko
Copy link
Contributor

@mgoworko mgoworko commented Dec 6, 2024

Describe your changes

Problem to be solved:

When fetching list of scenarios or status of all scenarios, for periodic scenarios there were always 2N db queries for N periodic scenarios. Those 2N queries were performed each time and endpoint /processes or /processes/status was called.

Solution:

  1. Defined capability of DeploymentManager (StateQueryForAllScenariosSupported). This capability means, that the DM can fetch all scenarios in a single operation.

  2. The new capability is implemented for PeriodicDeploymentManager and for FlinkStreamingDeploymentManagerProvider (which is an underlying DM of PeriodicDeploymentManager)

  3. When fetching processes or status, Nu uses the more efficient implementation of StateQueryForAllScenariosSupported when it is present (and it is present for periodics).

  4. This improvement can be implemented for other DM's too, but it is outside of the scope of this task. Moreover, we only have performance issues for periodic DM ATM.

Checklist before merge

  • Related issue ID is placed at the beginning of PR title in [brackets] (can be GH issue or Nu Jira issue)
  • Code is cleaned from temporary changes and commented out lines
  • Parts of the code that are not easy to understand are documented in the code
  • Changes are covered by automated tests
  • Showcase in dev-application.conf added to demonstrate the feature
  • Documentation added or updated
  • Added entry in Changelog.md describing the change from the perspective of a public distribution user
  • Added MigrationGuide.md entry in the appropriate subcategory if introducing a breaking change
  • Verify that PR will be squashed during merge

…deployment-manger-db-queries

# Conflicts:
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/DeploymentActor.scala
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/PeriodicDeploymentManager.scala
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/PeriodicProcessService.scala
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/db/PeriodicProcessesRepository.scala
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/flink/FlinkJarManager.scala
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/model/DeploymentWithJarData.scala
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/model/PeriodicProcessDeployment.scala
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/model/SchedulesState.scala
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/service/AdditionalDeploymentDataProvider.scala
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/service/PeriodicProcessListener.scala
#	engine/flink/management/periodic/src/main/scala/pl/touk/nussknacker/engine/management/periodic/service/ProcessConfigEnricher.scala
#	engine/flink/management/periodic/src/test/scala/pl/touk/nussknacker/engine/management/periodic/DeploymentActorTest.scala
#	engine/flink/management/periodic/src/test/scala/pl/touk/nussknacker/engine/management/periodic/JarManagerStub.scala
#	engine/flink/management/periodic/src/test/scala/pl/touk/nussknacker/engine/management/periodic/PeriodicProcessDeploymentGen.scala
#	engine/flink/management/periodic/src/test/scala/pl/touk/nussknacker/engine/management/periodic/PeriodicProcessGen.scala
#	engine/flink/management/periodic/src/test/scala/pl/touk/nussknacker/engine/management/periodic/PeriodicProcessServiceTest.scala
#	engine/flink/management/periodic/src/test/scala/pl/touk/nussknacker/engine/management/periodic/db/InMemPeriodicProcessesRepository.scala
@mgoworko mgoworko changed the title Draft: Optimize periodic deployment db queries Add Periodic DM db query caching Dec 13, 2024
@TouK TouK deleted a comment from github-actions bot Dec 13, 2024
@mgoworko mgoworko marked this pull request as ready for review December 13, 2024 12:09
Copy link
Contributor

created: #7358
⚠️ Be careful! Snapshot changes are not necessarily the cause of the error. Check the logs.

@mgoworko mgoworko changed the title Add Periodic DM db query caching Fetch status of all scenarios in DM in a single operation instead of separately for each scenario. Dec 18, 2024
@mgoworko mgoworko requested a review from arkadius December 19, 2024 13:03
Copy link
Member

@arkadius arkadius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mgoworko mgoworko merged commit 90e00a9 into staging Dec 19, 2024
17 checks passed
@mgoworko mgoworko deleted the optimize-periodic-deployment-manger-db-queries branch December 19, 2024 15:10
lciolecki pushed a commit that referenced this pull request Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants